test: fix flaky thread-safety test, update README tech stack by laurentiu021 · Pull Request #433 · laurentiu021/SystemManager

laurentiu021 · 2026-05-19T06:41:27Z

Summary

Test fix

OperationLockServiceTests.TryAcquire_IsThreadSafe — replaced timing-dependent Barrier + Thread.Sleep(50) with deterministic CountdownEvent + ManualResetEventSlim. Now asserts exactly 1 acquisition (was >= 1). Eliminates flaky failures on slow CI runners.

Documentation

README.md tech stack — added Microsoft.Extensions.DependencyInjection, H.NotifyIcon.Wpf, NSubstitute.

Summary by CodeRabbit

Documentation
- Updated README to include missing tech stack entries.
Tests
- Improved reliability of the operation-lock test by removing flaky timing, using deterministic synchronization, and tightening the assertion to ensure exactly one successful lock acquisition.

coderabbitai · 2026-05-19T06:41:40Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1c4acb2a-3717-4783-b3af-61541045ad83

📥 Commits

Reviewing files that changed from the base of the PR and between 48a70e3 and 2fb22aa.

📒 Files selected for processing (1)

SysManager/SysManager.Tests/OperationLockServiceTests.cs

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Build & unit tests
GitHub Check: Analyze (csharp)

🔇 Additional comments (1)

SysManager/SysManager.Tests/OperationLockServiceTests.cs (1)

96-99: Dispose synchronization primitives in this test scope.

This is still unresolved: CountdownEvent/ManualResetEventSlim created here should be disposed (ready, go, and allTried) to avoid handle leaks in test runs.

📝 Walkthrough

Walkthrough

This PR makes the TryAcquire_IsThreadSafe test deterministic by replacing Barrier + Thread.Sleep with CountdownEvent + ManualResetEventSlim, tightening the assertion to expect exactly one acquisition, and updates CHANGELOG/README to document the test fix and added tech-stack entries.

Changes

Test Determinism and Documentation

Layer / File(s)	Summary
Test Synchronization Determinism `SysManager/SysManager.Tests/OperationLockServiceTests.cs`	`TryAcquire_IsThreadSafe` replaces `Barrier` coordination with `CountdownEvent` + `ManualResetEventSlim` to synchronize 10 concurrent workers, changes the operation category to `SystemModification`, removes `Thread.Sleep`, and tightens the assertion to `Assert.Equal(1, successCount)` with `Interlocked` counting.
Documentation Updates `CHANGELOG.md`, `README.md`	CHANGELOG.md records the deterministic test synchronization fix and stricter assertion. README.md tech stack adds `Microsoft.Extensions.DependencyInjection` and includes `NSubstitute` in the test tooling list alongside `xUnit` and `FlaUI`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

laurentiu021/SystemManager#374: Related change that rewrote OperationLockService.TryAcquire/Release to use ConcurrentDictionary atomic TryAdd/TryRemove, which is relevant to the test's tightened single-acquisition assertion.

Poem

🐰 A test once slept while threads would pry,
Now events align—no flake nor lie,
One lock, one hop, the race is done,
Docs sing out the stack that's spun! 🌿✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the two main changes: fixing a flaky thread-safety test and updating the README tech stack documentation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/test-quality-and-docs

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@SysManager/SysManager.Tests/OperationLockServiceTests.cs`:
- Around line 96-97: The test creates CountdownEvent (ready) and
ManualResetEventSlim (go) which implement IDisposable and must be disposed to
avoid handle leaks; update the test in OperationLockServiceTests (the scope
where ready and go are created) to ensure both ready and go are disposed—either
wrap their creation in using blocks (or nested using statements) or dispose them
in a finally/tear-down block so ready.Dispose() and go.Dispose() are always
called after the test completes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d8781a2-c3aa-4d27-8640-344773c4feb8

📥 Commits

Reviewing files that changed from the base of the PR and between 9628cb0 and 48a70e3.

📒 Files selected for processing (3)

CHANGELOG.md
README.md
SysManager/SysManager.Tests/OperationLockServiceTests.cs

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Build & unit tests
GitHub Check: Analyze (csharp)

🔇 Additional comments (3)

SysManager/SysManager.Tests/OperationLockServiceTests.cs (1)

99-114: LGTM!

README.md (1)

455-455: LGTM!

Also applies to: 458-458, 460-460

CHANGELOG.md (1)

9-17: LGTM!

coderabbitai · 2026-05-19T06:43:52Z

+        var ready = new CountdownEvent(10);
+        var go = new ManualResetEventSlim(false);


⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Dispose synchronization primitives to prevent handle leaks.

CountdownEvent and ManualResetEventSlim both implement IDisposable and should be disposed to release their underlying kernel handles.

🛠️ Proposed fix

- var ready = new CountdownEvent(10); - var go = new ManualResetEventSlim(false); + using var ready = new CountdownEvent(10); + using var go = new ManualResetEventSlim(false);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

var ready = new CountdownEvent(10);

var go = new ManualResetEventSlim(false);

using var ready = new CountdownEvent(10);

using var go = new ManualResetEventSlim(false);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SysManager/SysManager.Tests/OperationLockServiceTests.cs` around lines 96 - 97, The test creates CountdownEvent (ready) and ManualResetEventSlim (go) which implement IDisposable and must be disposed to avoid handle leaks; update the test in OperationLockServiceTests (the scope where ready and go are created) to ensure both ready and go are disposed—either wrap their creation in using blocks (or nested using statements) or dispose them in a finally/tear-down block so ready.Dispose() and go.Dispose() are always called after the test completes.

…se positives

#433) ## Summary **Batch 9** — accessibility improvements across all XAML views. ### Issue #411 — Accessibility missing **Emoji replaced with text** across 21 XAML views: - \⟳\ refresh arrows → removed (text already says Refresh/Scan/Rescan) - \📁\ folder icons → Browse/Open text - \🔍\ search icons → removed (text already says Analyze/Scan) - \✕\ close/cancel → Cancel/X text - \📂\ open folder → Open text - \📋\ clipboard → Copy text - \🗑\ trash → removed (text already says Uninstall) - \⬆\ up arrow → Up text - \↺\ restore → removed (text already says Restore All) **AutomationProperties.Name added:** - All DataGrid elements → \Data table\ - ProgressBar elements → \Progress\ Screen readers now get meaningful text instead of emoji Unicode descriptions. ### Testing - Build: 0 errors - 21 files changed, 65 insertions, 65 deletions (net zero — replacements only) Closes #411 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>

* test: fix flaky thread-safety test, update README tech stack * test: hold lock during concurrent acquisition attempts to prevent false positives --------- Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>

test: fix flaky thread-safety test, update README tech stack

48a70e3

coderabbitai Bot reviewed May 19, 2026

View reviewed changes

test: hold lock during concurrent acquisition attempts to prevent fal…

2fb22aa

…se positives

laurentiu021 merged commit 9a4c197 into main May 19, 2026
5 checks passed

laurentiu021 deleted the fix/test-quality-and-docs branch May 19, 2026 06:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: fix flaky thread-safety test, update README tech stack - #433

test: fix flaky thread-safety test, update README tech stack#433
laurentiu021 merged 2 commits into
mainfrom
fix/test-quality-and-docs

laurentiu021 commented May 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 19, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		var ready = new CountdownEvent(10);
		var go = new ManualResetEventSlim(false);

Uh oh!

Conversation

laurentiu021 commented May 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test fix

Documentation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

laurentiu021 commented May 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 19, 2026 •

edited

Loading